Skip to content

Remove parse_uri, switch to using Uri module instead #5775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 4, 2024
Merged

Remove parse_uri, switch to using Uri module instead #5775

merged 2 commits into from
Jul 4, 2024

Conversation

last-genius
Copy link
Contributor

@last-genius last-genius commented Jul 4, 2024 •
edited by psafont
Loading

Follows on from #5726.

Introduces a fix to a bug noticed by a failing quicktest (CA-395184):

diff --git a/ocaml/libs/http-lib/http_svr.ml b/ocaml/libs/http-lib/http_svr.ml
index 2950bb3f7..c27f59a79 100644
--- a/ocaml/libs/http-lib/http_svr.ml
+++ b/ocaml/libs/http-lib/http_svr.ml
@@ -375,7 +375,7 @@ let request_of_bio_exn ~proxy_seen ~read_timeout ~total_timeout ~max_length bio
                  (* Request-Line   = Method SP Request-URI SP HTTP-Version CRLF *)
                  let uri_t = Uri.of_string uri in
                  if uri_t = Uri.empty then raise Http_parse_failure ;
-                 let uri = Uri.path uri_t in
+                 let uri = Uri.path uri_t |> Uri.pct_decode in
                  let query = Uri.query uri_t |> kvlist_flatten in
                  let m = Http.method_t_of_string meth in
                  let version =

Query doesn't need to be percent-decoded, the problem is in Uri.path:

utop # Uri.of_string "/foo?x=<>`&" |> Uri.query;;
- : (string * string list) list = [("x", ["<>`"]); ("", [])]
utop # Uri.of_string "/foo<>`&?x=<>`&" |> Uri.path;;
- : string = "/foo%3C%3E%60&"
utop # Uri.of_string "/foo<>`&?x=<>`&" |> Uri.path |> Uri.pct_decode;;
- : string = "/foo<>`&"

=====

This change passed the BST+BVT test suites

Andrii Sultanov added 2 commits June 26, 2024 14:17
CP-49953: Remove parse_uri, switch to using Uri module instead
de02e53 
Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
Fix a bug noticed by a quicktest run
e53ce67 
Introduces percent-decoding back - in the past we used to do
urlencode in parse_uri instead.

Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
@coveralls
Copy link

Coverage Status

coverage: 44.887%. remained the same
when pulling e53ce67 on last-genius:private/asultanov/uri-improvement
into e61e0ac on xapi-project:master.

edwintorok
edwintorok approved these changes Jul 4, 2024
View reviewed changes
@robhoes robhoes merged commit 6dd7a48 into xapi-project:master Jul 4, 2024
15 checks passed
@last-genius last-genius deleted the private/asultanov/uri-improvement branch July 10, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robhoes robhoes robhoes approved these changes

@edwintorok edwintorok edwintorok approved these changes

@psafont psafont psafont approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@last-genius @coveralls @robhoes @edwintorok @psafont